Skip to content

Add configuration support for custom DAG importers - #72442

Closed
dilnazanlid wants to merge 4 commits into
apache:mainfrom
dilnazanlid:aip-85-airflow-config
Closed

dilnazanlid wants to merge 4 commits into
apache:mainfrom
dilnazanlid:aip-85-airflow-config

Conversation

@dilnazanlid

@dilnazanlid dilnazanlid commented Sep 2, 2026 •

Copy link
Copy Markdown
Contributor

This pull request was one of the incremental parts of the AIP-85: DAG Importer implementation. See closing comment.

Description

This pull request is part of the implementation of AIP-85: DAG Importer. It introduces configuration-driven registration and resolution for custom Dag importers at both the global and Dag bundle levels.

Prior to this change, Dag file discovery and parsing were tied to built-in Python file discovery (list_py_file_paths), and the DagImporterRegistry operated strictly as a singleton with hardcoded defaults. This PR introduces a configurable, tiered Dag importer resolution mechanism:

  1. Shared Module Loading (airflow_shared.module_loading.dag_importers):

    • Added load_dag_importers() utility to dynamically instantiate importer classes via classpath and kwargs.
    • Supports explicit extensions configuration per importer, falling back to the importer's supported_extensions attribute or method.
    • Validates configuration formats and raises AirflowConfigException on malformed inputs.
  2. 3-Tier Importer Registry Precedence in Dag Bundles (DagBundlesManager):

    • Extended Dag bundle schemas (_ExternalBundleConfig, _InternalBundleConfig) to support an optional importers list.
    • Established precedence hierarchy when building each bundle's DagImporterRegistry:
      1. Bundle configuration (bundle.importers): Importers explicitly declared for that specific bundle (highest priority).
      2. Global configuration ([dag_processor] dag_importer_configs): System-wide custom importers configured via Airflow config.
      3. Default built-ins: Standard PythonDagImporter handling .py and .zip files (baseline fallback).
    • Added bundle.importer_registry property to BaseDagBundle and cached per-bundle registries in DagBundlesManager.
  3. Instantiable DagImporterRegistry:

    • Refactored DagImporterRegistry from a rigid process-wide singleton into an instantiable class, allowing isolated registries per Dag bundle while preserving get_importer_registry() for global access.
    • Enhanced register() to allow custom extension lists and log informative warnings when an extension mapping is overridden by a higher-priority importer.
  4. Integration with File Discovery and DagBag:

    • Updated DagFileProcessorManager._find_files_in_bundle() to discover Dag files dynamically using bundle.importer_registry.list_dag_files().
    • Updated DagBag to accept and utilize bundle-scoped importer_registry instances during Dag parsing and collection.
  5. Tests:

    • Added unit tests for dynamic importer loading and validation in airflow_shared.module_loading.
    • Added tests in test_dag_bundle_manager.py verifying the 3-tier precedence rules, extension overrides, nested composite importers, and error handling.
    • Updated test_registry.py and test_manager.py to cover independent registry instances and bundle discovery.

Was generative AI tooling used to co-author this PR?
  • Yes

Generated-by: Google Gemini following the guidelines

@SameerMesiah97 SameerMesiah97 left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not an expert here but this looks fairly clean from my perspective but we need to wait for CI to be triggered as there is a high risk of breaking changes here. I have left some comments.

Comment thread airflow-core/tests/unit/dag_processing/bundles/test_dag_bundle_manager.py Outdated
Comment thread airflow-core/tests/unit/dag_processing/bundles/test_dag_bundle_manager.py Outdated
Comment thread airflow-core/src/airflow/dag_processing/bundles/manager.py Outdated
Comment thread airflow-core/src/airflow/dag_processing/manager.py Outdated

@jason810496 jason810496 left a comment •

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice! Thanks for the PR, would it be better to encapsulate the logic for handling different level of importer configs into the DagImporterRegistry?

IIUC, the lifecycle of the DagImporterRegistry should be similar as the CoordinatorManager, so I prefer to leverage the @functools.cache to ensure the singleton and mirror the lazy import for those actual importer class.

https://github.com/apache/airflow/blob/b3b62fa8cf3c32cb45bb2815385a2af498300754/task-sdk/src/airflow/sdk/execution_time/coordinator.py

Comment thread airflow-core/src/airflow/dag_processing/importers/base.py Outdated
Comment thread airflow-core/src/airflow/dag_processing/bundles/base.py Outdated
Comment on lines +714 to +720
def get_importer_registry(self, bundle_name: str) -> DagImporterRegistry:
"""Get the DAG importer registry for a bundle."""
if bundle_name not in self._bundle_importers:
cfg = self._bundle_config.get(bundle_name)
importers_config = cfg.importers if cfg else None
self._bundle_importers[bundle_name] = self.create_importer_registry(bundle_name, importers_config)
return self._bundle_importers[bundle_name]

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

IIUC, the purpose of coupling DagImporterRegistry and DagBundlesManager is re-using the _bundle_config.

Would it be better to introduce a common layer for retrieving the configs like -- https://github.com/apache/airflow/pull/70805/changes#diff-1232041046ad2f8635ae867e2cf30914e6ad4619401897d723f6e57731c05e7bR118-R136.
Or wait until #70805 first, then make the _load_bundle_config_snapshot a public function then we don't need to couple DagImporterRegistry and DagBundlesManager together.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Correct, it was for the _bundle_config and parsing of the importer classes in the bundle manager. I added the similar code as in #70805 in here too, to check it works correctly. Overall, it would be good to submit this(and other 2 PRs) because there is last PR that will need all and use importers from SDK and move the filesystem path to dag definition.

Do you think your change is going to be submitted soon?

Comment thread airflow-core/src/airflow/dag_processing/manager.py Outdated
@dilnazanlid
dilnazanlid force-pushed the aip-85-airflow-config branch 2 times, most recently from 6cec340 to fdfb6da Compare September 8, 2026 14:39
@jason810496 jason810496 moved this from Todo to In Progress in AIP-85 Dag Importer Sep 9, 2026
@dilnazanlid dilnazanlid changed the title [AIP-85] Add configuration support for custom DAG importers Add configuration support for custom DAG importers Sep 14, 2026
…d extensions reach parsing instead of being ignored by hardcoded defaults.
Avoid premature class loading and tight coupling between bundles and
importer registries by delegating 3-tier configuration resolution to
DagImporterRegistry with cached lazy instantiation.
@dilnazanlid
dilnazanlid force-pushed the aip-85-airflow-config branch 2 times, most recently from db3a2a0 to 1015e86 Compare September 14, 2026 14:41
@dilnazanlid
dilnazanlid marked this pull request as draft September 14, 2026 14:43
@dilnazanlid

Copy link
Copy Markdown
Contributor Author

Closing this in favour of a single change where we replace the dag_processing/importers to sdk/importers.

This PR introduces the [dag_processor] dag_importer_configs option and the
per-bundle importers key, and wires them into airflow.dag_processing.importers.
That package is transitional: AIP-85 replaces it with airflow.sdk.importers,
which already implements this exact config parsing in
DagImporterRegistry.from_config() and _get_bundle_importers_config().

So landing the config here means (a) building plumbing in a package that the very
next PR deletes, and (b) shipping an interim state where one option is read by two
different registries. Neither is worth the review cycles.

The follow-up PR will instead do it in one step:

  • declare [dag_processor] dag_importer_configs and the per-bundle importers
    key in config.yml,
  • wire airflow.sdk.importers into the Dag processor,
  • remove airflow.dag_processing.importers.

All the discussed points here were already accommodated in the sdk/importers in #72369. Thanks!

@github-project-automation github-project-automation Bot moved this from In Progress to Done in AIP-85 Dag Importer Sep 14, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

Development

Successfully merging this pull request may close these issues.

3 participants